Skip to content

Conversation

Samuel-StO
Copy link
Contributor

@Samuel-StO Samuel-StO commented Sep 1, 2025

Context

In the Playwright helper, startRecordingTraffic() attaches a listener on the requestfinished event to capture completed network requests.
However, stopRecordingTraffic() was calling removeAllListeners('request'), which does not remove requestfinished listeners. This traffic is still recording event after the stopRecordingTraffic.

Also, in seeTraffic, the condition was too strict: it required both recording === true and recordedAtLeastOnce. This prevented assertions on traffic after stopRecordingTraffic().

Fix

  • Updated stopRecordingTraffic() to remove listeners for both request and requestfinished.
    • Instead of changing the event listened to in startRecordingTraffic(), I chose to keep requestfinished and just ensure proper cleanup. This minimizes changes and avoids breaking compatibility.
  • Simplified seeTraffic condition: now it only checks recordedAtLeastOnce. Users can assert collected traffic even after recording is stopped.

Tests

  • Added a test ensuring that after calling stopRecordingTraffic(), new requests are no longer collected.

Impact

  • Keeps actions.js generic and safe.
  • No API changes.
  • Fixes stop recordings and makes seeTraffic usable after stopping traffic recording.

Applicable helpers:

  • Playwright
  • Puppeteer
  • WebDriver
  • REST
  • FileHelper
  • Appium
  • TestCafe

Applicable plugins:

  • allure
  • autoDelay
  • autoLogin
  • customLocator
  • pauseOnFail
  • coverage
  • retryFailedStep
  • screenshotOnFail
  • selenoid
  • stepByStepReport
  • stepTimeout
  • wdio
  • subtitles

Type of change

  • 🔥 Breaking changes
  • 🚀 New functionality
  • 🐛 Bug fix
  • 🧹 Chore
  • 📋 Documentation changes/updates
  • ♨️ Hot fix
  • 🔨 Markdown files fix - not related to source code
  • 💅 Polish code

Checklist:

  • Tests have been added
  • Documentation has been added (Run npm run docs)
  • Lint checking (Run npm run lint)
  • Local tests are passed (Run npm test)

@Samuel-StO
Copy link
Contributor Author

Hmm, the tests are failing on Puppeteer. I’ll take a look tomorrow to try to understand why. Do you have any idea why the second amOnPage doesn’t pass? I’m not very familiar with Puppeteer.

@Samuel-StO
Copy link
Contributor Author

For Puppeteer, I think I may have identified the issue, but I’d like confirmation since I’m not an expert in Puppeteer or the Codecept recorder.

In this helper we set await this.page.setRequestInterception(true):

async startRecordingTraffic() {
this.flushNetworkTraffics()
this.recording = true
this.recordedAtLeastOnce = true
await this.page.setRequestInterception(true)
this.page.on('request', request => {
const information = {

However, when we stop, we never set this value back to false:

stopRecordingTraffic() {
stopRecordingTraffic.call(this)
}

My proposal is to modify it like this:

async stopRecordingTraffic() {
  await this.page.setRequestInterception(false);
  stopRecordingTraffic.call(this);
}

From what I’ve read, if requestInterception is set to true, we must always handle the requests. That means we should reset it to false when we flush the listeners: puppeteer/puppeteer#2055 (comment)

The tests passed with this modification, but I have one last question: what would be the impact of making stopRecordingTraffic asynchronous?

@kobenguyent
Copy link
Collaborator

For Puppeteer, I think I may have identified the issue, but I’d like confirmation since I’m not an expert in Puppeteer or the Codecept recorder.

In this helper we set await this.page.setRequestInterception(true):

async startRecordingTraffic() {
this.flushNetworkTraffics()
this.recording = true
this.recordedAtLeastOnce = true
await this.page.setRequestInterception(true)
this.page.on('request', request => {
const information = {

However, when we stop, we never set this value back to false:

stopRecordingTraffic() {
stopRecordingTraffic.call(this)
}

My proposal is to modify it like this:

async stopRecordingTraffic() {
  await this.page.setRequestInterception(false);
  stopRecordingTraffic.call(this);
}

From what I’ve read, if requestInterception is set to true, we must always handle the requests. That means we should reset it to false when we flush the listeners: puppeteer/puppeteer#2055 (comment)

The tests passed with this modification, but I have one last question: what would be the impact of making stopRecordingTraffic asynchronous?

thanks for pointing it out @Samuel-StO, maybe it's a missing piece when trying to consolidate the actions to be shared among Puppeteer, Playwright and WebDriver helpers -> https://github.com/codeceptjs/CodeceptJS/pull/4263/files#diff-8b5994d9478083cafc2a9a5943b10cbff2648e67b092689c43e5f3d8924b4096R107

@kobenguyent kobenguyent merged commit a396c5b into codeceptjs:3.x Sep 2, 2025
14 checks passed
@Samuel-StO Samuel-StO deleted the bugfix/fix-traffic-recording branch September 2, 2025 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants